One run, one answer about itself (#104) - #452
Merged
Merged
Conversation
`{{ execution.timestamp }}` had seven implementations in four formats:
orchestrator.py:307 %Y-%m-%d-%H:%M:%S
orchestrator.py:1424, :1994 .isoformat()
control_system.py:222 %Y-%m-%d %H:%M:%S
hybrid_control_system.py:594 %Y-%m-%d %H:%M:%S
hybrid_control_system.py:325 %Y-%m-%dT%H:%M:%S, plus iso_timestamp,
pipeline_id and execution_id
declarative_engine.py:121 no timestamp at all -- start_time
They did not merely disagree between engines. `_execute_level` rebuilt the
dict at every level of the graph, overwriting what the run had registered,
so one run answered its own question differently each time:
step one -> 2026-08-02T20:01:55.182681
step two -> 2026-08-02T20:01:55.184368
Anything naming an output file after the timestamp wrote several.
Meanwhile `validate` rejected the expression outright -- 59 references
across 32 catalogue pipelines, every one of which ran correctly and failed
validation. My earlier note called `execution` unpopulated; it is populated,
and that was wrong.
The namespace was both too permissive and too strict, in different
validators. The data-flow validator accepted `execution.anything` plus
`pipeline`, `context` and `env`, which nothing populates; the template
validator rejected the base name, so pipelines were refused anyway and the
permissiveness sat unnoticed. Both now read one schema.
execution.id run-4f2a91c07e3b
execution.started_at 2026-01-15T14:30:45+00:00
execution.timestamp (the same instant under its older name)
execution.date 2026-01-15
execution.time 14:30:45
`date` and `time` are beyond the three fields the review specified. They
work today and two catalogue files use `date`, so dropping them would have
removed working behaviour from a hardening change. Everything else is
refused: `{{ execution.strated_at }}` is a typo, not a field, and an open
namespace would render it as an empty string and report success.
UTC, so stamps from two machines compare and a run spanning a
daylight-saving change does not go backwards.
The cached value is a plain dict: caching the `RuntimeContext` object made
every checkpointed run fail with "Object of type RuntimeContext is not JSON
serializable".
Measured: catalogue validating 20 -> 30 of 117, the largest jump so far;
blocking suite 670 -> 696 passed, 0 failed.
Five mutations, all killed:
- rebuild the namespace per level -> the stability test
- restore the wildcard namespace accept -> 7 tests
- accept any execution field -> 3 tests
- template validator forgets the namespace -> 5 tests
- local time instead of UTC -> 1 test
The wildcard mutation survived twice before it spoke. First because every
end-to-end test rejects `pipeline.name` through the *template* validator
whether or not the data-flow validator accepts it -- so the line I changed
had no test at all, which is exactly why the permissiveness survived this
long. Then because my first attempt at the mutation was not a faithful
inverse: it routed the phantom namespaces through the field check, which
still rejected them. The direct unit tests are the ones that hold it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`now()` reads the clock afresh at every use, so two steps of one run get
different answers. `execution.timestamp` is computed once per run. Six
catalogue examples stamped their reports with `now()`, which is exactly the
case where one answer is wanted:
"timestamp": "{{ now() }}" -> "{{ execution.timestamp }}"
*Report generated on: {{ now() }}*
A warning, not an error. `now()` keeps working -- pipelines outside this
repository use it, and refusing them to make a style point would be a
gratuitous break -- but validation now names the replacement:
'now()' is deprecated: it is read afresh at every use, so two steps of
one run disagree. Use 'execution.timestamp', which is the same for
every step.
The pipeline still validates (exit 0) and still runs.
`GlobalSpec` carries `deprecated_for`, so the message, the generated
documentation and the catalogue guard all come from one declaration rather
than three restatements. `find_global_misuse` reports it only for a call
that would otherwise work: a wrong-arity `now(1, 2)` gets the arity error
alone, one problem at a time.
`TemplateValidationError` already carried a severity; the validator now
routes on it, so a warning goes to `warnings` and lets the run proceed while
an error still refuses it.
Measured: blocking suite 696 -> 700 passed, 0 failed. Catalogue unchanged at
30/117 -- the six migrated files fail for unrelated reasons, so this moves
correctness rather than the count.
Three mutations, all killed:
- drop `deprecated_for` from `now` -> 2 tests
- make the deprecation an error -> 4 tests
- report it for a call that cannot work anyway -> 1 test
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Deprecate now() in favour of execution.timestamp (#104)
This was referenced Aug 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Step 2 of the recommended sequence: the typed runtime context.
Stacked on #451 — review that first; the base will retarget to
mainwhen it merges.The defect, measured
Seven sites built
executionin four timestamp formats — and an eighth field set I found while wiring it (hybrid._get_execution_metadataalso emittediso_timestamp,pipeline_id,execution_id).Worse than cross-engine disagreement:
_execute_levelrebuilt the dict at every level of the graph, overwriting what the run had registered. One run, two steps:Anything naming an output file after the timestamp wrote several. Now:
I was wrong earlier, and it changed the plan
My note from the #448 diagnosis said
executionwas never populated. It is. And the "rejectpipeline/context/env" requirement was already the behaviour — the data-flow validator accepted them, but the template validator rejected the base name, so pipelines were refused anyway. That permissiveness was invisible, not harmless.The live defect was the opposite:
execution.timestampran correctly and failed validation, across 59 references in 32 files.execution.timestampexecution.id/started_at/date/timeexecution.strated_atpipeline.name,context.foo,env.HOMEvalidateandrunnow agree on every case.One deviation, taken deliberately
Your schema was
id/started_at/timestamp. I also exposedateandtime, because both work today and two catalogue files usedate— dropping them would have removed working behaviour inside a hardening change. Everything else is refused. Say the word and they go, plus two example migrations.Measured
Mutations — five, all killed
executionfieldThe wildcard mutation survived twice before it spoke. First because every end-to-end test rejects
pipeline.namevia the template validator whether or not the data-flow validator accepts it — so the line I changed had no test at all, which is precisely why the permissiveness survived this long. Then because my first mutation wasn't a faithful inverse: it routed the phantom namespaces through the field check, which still rejected them. Four direct unit tests now hold that line.Two other self-inflicted problems caught before commit: caching the
RuntimeContextobject broke every checkpointed run (not JSON serializable), and the generated doc embedded a live clock, so its--checktest would have failed on every regeneration.Not in this PR
now()deprecation and the catalogue migration toexecution.timestamp; the secrets contract for the singleenvexample.now's doc entry already points atexecution.timestampfor the deterministic case.🤖 Generated with Claude Code